-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Webview navigation history getter #72
base: nw22
Are you sure you want to change the base?
Conversation
c286db2
to
aa1726c
Compare
… not work for images and videos For links of images and videos, the options 'Open in new Chrome tab' and 'Open in Chrome incognito tab' under context menu do not work, because the linkUrl is empty. To fix this, check whether the link is video or image, if so, pass the srcUrl instead of linkUrl. Also for downloaded images and videos, the context menu should be disabled because current options ('Share' and 'Download image/video') could be achieved through overflow menu. To fix this, check if the srcUrl starts with 'file://'. BUG=696417 NOTRY=true NOPRESUBMIT=true Original-Review-Url: https://codereview.chromium.org/2725473002 Cr-Original-Commit-Position: refs/heads/master@{#455181} (cherry picked from commit 2fc04cd) Review-Url: https://codereview.chromium.org/2739723004 Cr-Commit-Position: refs/branch-heads/3029@{#72} Cr-Branched-From: 939b32e-refs/heads/master@{#454471}
65c8d20
to
482e210
Compare
@@ -148,11 +148,12 @@ WebViewImpl.prototype.onFrameNameChanged = function(name) { | |||
// Updates state upon loadcommit. | |||
WebViewImpl.prototype.onLoadCommit = function( | |||
baseUrlForDataUrl, currentEntryIndex, entryCount, | |||
processId, url, isTopLevel) { | |||
processId, url, isTopLevel, pagesHistory) { | |||
this.baseUrlForDataUrl = baseUrlForDataUrl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would send the whole history from browser to renderer process on every navigation, which could hurt performance. Is there any better way to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s true it is send with every navigation. But I consider it to be insignificant. After all it is just few strings. I guess it would be possible to make changes just in javascript code. Even without ‘pagesHistory’ you get ‘url’ and ‘currentEntryIndex’ from which you can create array of everything you need. But then all logic needs to be there. E.g. if you go back and then navigate to other page you should delete some record.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The history will grow longer and longer. So eventually the data will grow big enough to hurt performance. And there is a size limit of IPC message. Should find another way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t know about limit for IPC message. But it will not grow longer and longer. For navigation history there is some limit. When you have about 30 or so items it will remove the oldest. Or at least it seems to be the case after quick test.
426fc70
to
f78c16a
Compare
da3929b
to
b060cdf
Compare
f464f22
to
1884f31
Compare
Updated to current nw21. so it will not grow longer and longer. In some cases, even 50 entries could be quite big. But we never encountered any issue related to this. Regarding the performance, I don't think it is significant. Currently we send everything at once synchronously. This suit us well since we use it to show session history. We could send it one at a time asynchronously but I guess that would be even more performance demanding. Other way could be to restrict it more e.g. to last 10 entries. On the other hand, if use case would be to get just one entry at a time it seems to be Ok. |
9396d7f
to
8adf1c9
Compare
f1e4ded
to
4fd96a3
Compare
27692f7
to
f854061
Compare
BUG=711612 Review-Url: https://codereview.chromium.org/2818153002 Cr-Commit-Position: refs/heads/master@{#464763} (cherry picked from commit 3b1c2e7) Review-Url: https://codereview.chromium.org/2827173002 . Cr-Commit-Position: refs/branch-heads/3071@{#72} Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
Webview now has: -getPagesHistory function, returning array of URLs, titles and favicons of pages in history. Titles and favicons are not known for current page because array is created before page is fully loaded. -getCurrentHistoryIndex function, returning current history index
65748ff
to
90fd209
Compare
Fixes a number of issues with stylus settings for note taking on lock screen: * Provide a NoteTakingHelper observer interface called when the preferred app changes (or when it's lock screen status changes) * Settings UI can use this to update itself when the preferred app changes. * Switch lock_screen_apps::AppManagerImpl to observe this event for preferred app changes (instead of observing note taking pref directly) * Introduces kNoteTakingAppsAllowedOnLockScreen pref, that will be used by a user policy to whitelist apps available on the lock screen (to be added in dependent patch) * Disable lock screen support for note taking apps in non-primary profiles (the profile that supports lock screen use case is set by lock_screen_apps::StateController during its initialization). * Redo settings UI for enabling apps on the lock screen so its state (whether it's disabled, the policy indicator) does not depend on prefs directly, instead derive the state from the note taking app's NoteAppInfo (in particular lockScreenSupport property) While here, did some cleanup in test code: * Provided utility methods to NoteTakingHelper unit tests to reduce code duplication: * Method to create/install lock screen enabled note taking app * Methods for verifying preferred app and available apps info * In stylus device page browser tests, made fake browser proxy smarter, so test don't have to "manually" trigger note taking app changes [email protected] (cherry picked from commit e4a5c06) Bug: 741940 Bug: 741053 Bug: 746116 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I5e2ee138df620d3832a8ad0d1b4d0db285fba0da Reviewed-on: https://chromium-review.googlesource.com/572842 Commit-Queue: Toni Barzic <[email protected]> Reviewed-by: Jacob Dufault <[email protected]> Reviewed-by: Steven Bennetts <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#489142} Reviewed-on: https://chromium-review.googlesource.com/588341 Reviewed-by: Toni Barzic <[email protected]> Cr-Commit-Position: refs/branch-heads/3163@{#72} Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
This reverts commit 9580368. Reason for revert: Suspect that there is a leak in this CL Original change's description: > [cronet] Re-use Direct ByteBuffer for Cronet upload > > This CL makes Cronet upload reuse a Java ByteBuffer object if the > underlying net::IOBuffer's address and buffer length are unchanged. > > This should reduce the number of constructor calls to > NewDirectByteBuffer(). > > Bug: 756841 > Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester > Change-Id: I751242095e2ba5793750d5a91e2bc3b10ec8b7a1 > Reviewed-on: https://chromium-review.googlesource.com/624196 > Reviewed-by: Andrei Kapishnikov <[email protected]> > Commit-Queue: Helen Li <[email protected]> > Cr-Commit-Position: refs/heads/master@{#496067} [email protected], [email protected] (cherry picked from commit f9121d1) Bug: 756841 Change-Id: I186de0df7b316e4284648b1b9cca1addc7f7845d Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester Reviewed-on: https://chromium-review.googlesource.com/656109 Reviewed-by: Helen Li <[email protected]> Commit-Queue: Helen Li <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#500411} Reviewed-on: https://chromium-review.googlesource.com/656002 Cr-Commit-Position: refs/branch-heads/3202@{#72} Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
To determine what printers users are having difficulty setting up, record the printer if setup is abandoned from the PPD selection screen. This will tell us where our PPD coverage is lacking. [email protected] (cherry picked from commit c397a59) Bug: 756576 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Id1e0913a3096d8a674a14cf230f6b97af456b330 Reviewed-on: https://chromium-review.googlesource.com/602696 Commit-Queue: Sean Kau <[email protected]> Reviewed-by: Demetrios Papadopoulos <[email protected]> Reviewed-by: Xiaoqian Dai <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#509654} Reviewed-on: https://chromium-review.googlesource.com/728348 Reviewed-by: Sean Kau <[email protected]> Cr-Commit-Position: refs/branch-heads/3239@{#72} Cr-Branched-From: adb61db-refs/heads/master@{#508578}
Restoring InspectorSession may send protocol notifications synchronously, so we should setup all bindings beforehand. [email protected] (cherry picked from commit c27cabf) Bug: 804214 Change-Id: I68989990fe210a61251dc20047b891fa4155d596 Reviewed-on: https://chromium-review.googlesource.com/879137 Reviewed-by: Pavel Feldman <[email protected]> Commit-Queue: Dmitry Gozman <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#531349} Reviewed-on: https://chromium-review.googlesource.com/884510 Reviewed-by: Dmitry Gozman <[email protected]> Cr-Commit-Position: refs/branch-heads/3325@{#72} Cr-Branched-From: bc084a8-refs/heads/master@{#530369}
IOSUserEventService is not synchronous. To avoid to have the DCHECK in ConsentAuditor::RecordGaiaConsent() failing, this service should be created as soon as possible, to make sure it is correctly initialized before using it. With this fix, it is now possible to put back the DCHECK in ConsentAuditor::RecordGaiaConsent(). Bug: 819176 Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs Change-Id: I66be7843eb3cc73686c07b0ffdee232c90dad7dd Reviewed-on: https://chromium-review.googlesource.com/951484 Commit-Queue: Jérôme Lebel <[email protected]> Reviewed-by: Martin Šrámek <[email protected]> Reviewed-by: Sylvain Defresne <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#541123}(cherry picked from commit 1f2f87f) Reviewed-on: https://chromium-review.googlesource.com/953463 Reviewed-by: Jérôme Lebel <[email protected]> Cr-Commit-Position: refs/branch-heads/3359@{#72} Cr-Branched-From: 66afc5e-refs/heads/master@{#540276}
Change the ChromeOS Audio Player to no longer use panels, but behave as a regular app. Panels are now deprecated. See the bug for more details. Bug: 800990 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: Ia081ac12da133a31ca89719b501b373152a0441c Reviewed-on: https://chromium-review.googlesource.com/1011863 Reviewed-by: Ben Wells <[email protected]> Commit-Queue: Sasha Morrissey <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#550930}(cherry picked from commit 8b54e20) Reviewed-on: https://chromium-review.googlesource.com/1016140 Reviewed-by: Sasha Morrissey <[email protected]> Cr-Commit-Position: refs/branch-heads/3396@{#72} Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
A device that is enrolled pre-M68 may not be able to obtain an enrollment certificate until it is wiped, but can request a computation of its EID immediately and upload that to the management servers. BUG=chromium:840496 TEST=unit_tests Change-Id: Ib1c4d2652110c49d1370fcc0dfbcfddb336c2de9 Reviewed-on: https://chromium-review.googlesource.com/1069599 Reviewed-by: Pavol Marko <[email protected]> Reviewed-by: Darren Krahn <[email protected]> Reviewed-by: Maksim Ivanov <[email protected]> Commit-Queue: Yves Arrouye <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#561890}(cherry picked from commit b7bfd93) Reviewed-on: https://chromium-review.googlesource.com/1080971 Reviewed-by: Yves Arrouye <[email protected]> Cr-Commit-Position: refs/branch-heads/3440@{#72} Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
There is a nullptr exception connected to SendViewItems. This CL guesses a solution to this problem: 1. It's only called if the controller was checked to be null, so it must be in the implementation. 2. The view_ is synchronously constructed at construction time and can outside of tests never be null. 3. It's bound to web_contents_, so this also cannot be null. The only dereference left is the GetFocusedFrame() method which can be null. Therefore, accessing the correct frame moves to the driver-side which is much more appropriate for frame-based actions anyway. [email protected] (cherry picked from commit f178cf1) Bug: 865447 Change-Id: Ie5ea7aed980cdf53e784e5d918a9d54717ad6ab1 Reviewed-on: https://chromium-review.googlesource.com/1143477 Reviewed-by: Vasilii Sukhanov <[email protected]> Commit-Queue: Friedrich Horschig <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#577496} Reviewed-on: https://chromium-review.googlesource.com/1150229 Reviewed-by: Friedrich Horschig <[email protected]> Cr-Commit-Position: refs/branch-heads/3497@{#72} Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
[email protected] Change-Id: I9371fb574654fda477097dd156076be0c651cec6 Reviewed-on: https://chromium-review.googlesource.com/1208415 Reviewed-by: [email protected] <[email protected]> Cr-Commit-Position: refs/branch-heads/3538@{#72} Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
Previously, the SQL query included site icons that were NULL or empty, resulting in category tiles often having less than 4 site icons present. This fix skips the empty icons, and grabs the next non-empty ones. Change-Id: I314c78f1797769e598640d06162811e1672c2449 Reviewed-on: https://chromium-review.googlesource.com/c/1279155 Reviewed-by: Peter Williamson <[email protected]> Commit-Queue: Jonathan Freed <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#599347}(cherry picked from commit cdef234) Reviewed-on: https://chromium-review.googlesource.com/c/1285097 Reviewed-by: Cathy Li <[email protected]> Cr-Commit-Position: refs/branch-heads/3578@{#72} Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
Bug: 845472 Change-Id: I3afc1e03c280c88cce039ac8d4a6b288edd7be78 Reviewed-on: https://chromium-review.googlesource.com/c/1355173 Reviewed-by: Yi Su <[email protected]> Commit-Queue: Javier Ernesto Flores Robles <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#612608}(cherry picked from commit 39cf762) Reviewed-on: https://chromium-review.googlesource.com/c/1363176 Reviewed-by: Javier Ernesto Flores Robles <[email protected]> Cr-Commit-Position: refs/branch-heads/3626@{#72} Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
in OverviewSession::OnKeyEvent Bug: 925878 Test: covered by unittest Change-Id: I712acfcc7127e86e30d5d1be3ef8f86831c12368 Reviewed-on: https://chromium-review.googlesource.com/c/1440913 Reviewed-by: Sammie Quon <[email protected]> Commit-Queue: Mitsuru Oshima <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#626807}(cherry picked from commit 8d4ae6d) Reviewed-on: https://chromium-review.googlesource.com/c/1446404 Reviewed-by: Mitsuru Oshima <[email protected]> Cr-Commit-Position: refs/branch-heads/3683@{#72} Cr-Branched-From: e510299-refs/heads/master@{#625896}
The following sequence of events can cause a segfault: 1. ThrottlingURLLoader::Start is called to kick off a request 2. A throttle's WillStartRequest method redirects the request by changing ResourceRequest::url 3. ThrottlingURLLoader::StartNow is called, possibly after deferral 4. StartNow calls ThrottlingURLLoader::OnReceivedRedirect because the request URL was changed in step 2 5. A throttle's WillRedirectRequest method defers the redirect 6. When handling the defer, OnReceivedRedirect calls client_binding_.PauseIncomingMethodCallProcessing, which segaults because client_binding_ is unbound because the request never actually started Bug: 933538 Change-Id: I0f7033d159d34601421da781f7902b6ae207c58a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1511620 Reviewed-by: John Abd-El-Malek <[email protected]> Commit-Queue: Robbie McElrath <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#639178}(cherry picked from commit a86e86d90b35de30ed66b5f0337a082474a01913) Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1519287 Reviewed-by: Robbie McElrath <[email protected]> Cr-Commit-Position: refs/branch-heads/3729@{#72} Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
Documentation (see for details):
nwjs/nw.js#5752
Our project:
https://groups.google.com/forum/#!topic/nwjs-general/TmZUq-JHIuQ